Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename max_splits to max_history_splits, set default value to 1.0e7 #2954

Merged
merged 8 commits into from
Jun 18, 2024

Conversation

vanessalulla
Copy link
Contributor

@vanessalulla vanessalulla commented Apr 12, 2024

Description

It was noted that the two variables max_split and max_splits had very similar naming. Max_split is a member of the weight window object and limits the number of splits during a single check against the weight window. Max_splits is a member of the settings object and limits the number of times a history can be split. Max_splits has a notable effect on the effectiveness of the weight window while max_split does not.

It was also found that the default value of 1000 splits is not enough when you have many orders of magnitude decrease in your flux across your model.

This PR renames the variable max_splits variable to max_history_splits to make it more clear for users and sets the default value of max_history_splits to be 1.0e7.

Fixes #2774

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@paulromano paulromano requested a review from pshriwise May 2, 2024 14:38
@paulromano paulromano added this to the v0.15.0 milestone Jun 10, 2024
@pshriwise pshriwise force-pushed the max_history_splits_2774 branch from c67f2e4 to 04b4e6f Compare June 17, 2024 20:33
Copy link
Contributor

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a small change to add the max_splits property back to raise an AttributeError if used.

@paulromano paulromano enabled auto-merge (squash) June 18, 2024 02:28
@paulromano paulromano merged commit 97537d5 into openmc-dev:develop Jun 18, 2024
15 checks passed
church89 pushed a commit to openmsr/openmc that referenced this pull request Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weight windows max_split vs max_splits
3 participants